Skip to content

Conversation

@vjik
Copy link
Member

@vjik vjik commented Jan 17, 2025

Q A
Is bugfix?
New feature?
Breaks BC?

@vjik vjik added the status:code review The pull request needs review. label Jan 17, 2025
@codecov
Copy link

codecov bot commented Jan 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.81%. Comparing base (16a53fd) to head (fdff99b).
Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #137      +/-   ##
============================================
+ Coverage     77.08%   77.81%   +0.73%     
  Complexity      184      184              
============================================
  Files            14       14              
  Lines           576      586      +10     
============================================
+ Hits            444      456      +12     
+ Misses          132      130       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vjik vjik requested a review from a team January 17, 2025 13:05
Copy link
Member

@xepozz xepozz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the reason for such changes? will array $settings be deprecated for further versions?

@vjik
Copy link
Member Author

vjik commented Jan 17, 2025

what's the reason for such changes? will array $settings be deprecated for further versions?

Typed properties is better than array. Array is incosistent to other Yii3 code. $settings already marked as deprecated in this PR.

@vjik vjik requested a review from a team January 17, 2025 14:31
@vjik vjik merged commit cc8b2cc into master Jan 18, 2025
22 of 23 checks passed
@vjik vjik deleted the improve branch January 18, 2025 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:code review The pull request needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants